Feature/smp granular locks v4 #1154

sudeep-mohanty · 2024-10-09T13:20:58Z

This PR adds support for granular locking to the FreeRTOS kernel.

Description

Granular locking introduces the concept of having localized locks per kernel data group for SMP configuration. This method is an optional replacement of the existing kernel locks and is controlled by a new port layer configuration, viz., portUSING_GRANULAR_LOCKS. More details about the approach can be found here.

Test Steps

The implementation has been tested on Espressif SoC targets, viz., the ESP32 using the ESP-IDF framework.

1. Testing on an esp32 target

To test the implementation of the granular locking scheme on esp32, setup the ESP-IDF environment on your local machine. The steps to follow are listed in the Getting Started Guide.
Instead of the main ESP-IDF repository, the granular locks implementation resides in this froked repository - https://github.com/sudeep-mohanty/esp-idf.
Once you have cloned the forked repository and setup the ESP-IDF environment, change your branch to feat/granular_locks_tests.
To run the FreeRTOS unit tests, change the directory to components/freertos/test_apps/freertos where all the test cases are located.
Performance tests are located in the performance subfolder at the same location.
Setup the target device with the command idf.py seet-target esp32.
Select the Amazon FreeRTOS SMP kernel using the menuconfig options. To do this you must enter the command -idf.py menuconfig -> Component config -> FreeRTOS -> Kernel -> Run the Amazon SMP FreeRTOS kernel instead (FEATURE UNDER DEVELOPMENT). Save the configuration and exit the menuconfig.
Now, build and flash the unit test application with the command idf.py build flash monitor.
Once the app is up, you can enter the number of the test case and run the unity test case.

TODO

Test setup for other targets (Raspberry Pi Pico)
Generic target tests to be uploaded to the FreeRTOS repository.

Checklist:

I have tested my changes. No regression in existing tests.
I have modified and/or added unit-tests to cover the code changes in this Pull Request.

Related Issue

Closes Granular Lock for SMP #905

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

sudeep-mohanty · 2024-10-09T13:23:42Z

@chinglee-iot @aggarg This PR introduces the granular locks changes to the FreeRTOS kernel. Please have a look and we could have discussions/changes in this PR context. Thank you.

cc: @ESP-Marius @Dazza0

rawalexe · 2024-10-09T16:55:58Z

Thank you for your contribution, I'll forward this request to the team. There are few error in the PR can you please try fixing them

rawalexe · 2024-10-14T22:28:24Z

Hello @sudeep-mohanty, I am just following up if you had time to fix the build issues

sudeep-mohanty · 2024-10-15T08:26:38Z

@rawalexe Yes! I shall work on the failures and would also do some refactoring for an easier review process. For now, I've put this PR in draft.

ActoryOu · 2024-10-31T05:02:09Z

Hi @sudeep-mohanty,
Could you help check CI failing?

sudeep-mohanty · 2024-10-31T09:27:52Z

Hi @sudeep-mohanty, Could you help check CI failing?

Hi @ActoryOu, I've made some updates which should fix the CI failures however I could not understand why the link-verifier action fails. Seems more like a script failure to me. So this action would still fail. If you have more information on what is causing it, could you let me know and I shall fix it. Thanks.

sudeep-mohanty · 2024-11-01T10:57:39Z

Created a PR to fix the CI failure - FreeRTOS/FreeRTOS#1292

sonarqubecloud · 2024-11-04T15:38:43Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
6.3% Duplication on New Code

See analysis details on SonarCloud

timers.c

tasks.c

chinglee-iot · 2025-07-21T05:26:00Z

include/task.h

+#if ( portUSING_GRANULAR_LOCKS == 1 )
+    #define taskDATA_GROUP_ENTER_CRITICAL_FROM_ISR( pxISRSpinlock, puxSavedInterruptStatus ) \
+    do {                                                                                     \
+        *( puxSavedInterruptStatus ) = portSET_INTERRUPT_MASK_FROM_ISR();                    \
+        const BaseType_t xCoreID = ( BaseType_t ) portGET_CORE_ID();                         \
+        /* Take the ISR spinlock */                                                          \
+        portGET_SPINLOCK( xCoreID, ( portSPINLOCK_TYPE * ) pxISRSpinlock );                  \
+        /* Increment the critical nesting count */                                           \
+        portINCREMENT_CRITICAL_NESTING_COUNT( xCoreID );                                     \
+    } while( 0 )
+#endif /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */


Suggested change

#if ( portUSING_GRANULAR_LOCKS == 1 )

#define taskDATA_GROUP_ENTER_CRITICAL_FROM_ISR( pxISRSpinlock, puxSavedInterruptStatus ) \

do { \

*( puxSavedInterruptStatus ) = portSET_INTERRUPT_MASK_FROM_ISR(); \

const BaseType_t xCoreID = ( BaseType_t ) portGET_CORE_ID(); \

/* Take the ISR spinlock */ \

portGET_SPINLOCK( xCoreID, ( portSPINLOCK_TYPE * ) pxISRSpinlock ); \

/* Increment the critical nesting count */ \

portINCREMENT_CRITICAL_NESTING_COUNT( xCoreID ); \

} while( 0 )

#endif /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */

#if ( portUSING_GRANULAR_LOCKS == 1 )

#define taskDATA_GROUP_ENTER_CRITICAL_FROM_ISR( pxISRSpinlock, puxSavedInterruptStatus ) \

do { \

*( puxSavedInterruptStatus ) = portSET_INTERRUPT_MASK_FROM_ISR(); \

{ \

const BaseType_t xCoreID = ( BaseType_t ) portGET_CORE_ID(); \

/* Take the ISR spinlock */ \

portGET_SPINLOCK( xCoreID, ( portSPINLOCK_TYPE * ) ( pxISRSpinlock ) ); \

/* Increment the critical nesting count */ \

portINCREMENT_CRITICAL_NESTING_COUNT( xCoreID ); \

} \

} while( 0 )

#endif /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */

chinglee-iot · 2025-07-21T05:31:28Z

tasks.c

+    #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
+        UBaseType_t uxSavedInterruptStatus = kernelENTER_CRITICAL_FROM_ISR();
+    #endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
+


Suggested change

#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )

UBaseType_t uxSavedInterruptStatus = kernelENTER_CRITICAL_FROM_ISR();

#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */

#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )

UBaseType_t uxSavedInterruptStatus;

#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */

traceENTER_xTaskIncrementTick();

#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )

uxSavedInterruptStatus = kernelENTER_CRITICAL_FROM_ISR();

#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */

Suggestion for ISO C90 requirement that all variable declarations must be at the beginning of a block.

chinglee-iot · 2025-07-21T05:33:00Z

tasks.c

+         * called from a critical section within an ISR. */
+    #else /* #if ( ! ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) ) */
+        /* Lock the kernel data group as we are about to access its members */
+        UBaseType_t uxSavedInterruptStatus;


ISO C90 requires all variable declarations must be at the start of a block. Suggest we move the declaration before the trace macros.

chinglee-iot · 2025-07-21T05:34:22Z

timers.c

+    #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
+        PRIVILEGED_DATA static portSPINLOCK_TYPE xTaskSpinlock = portINIT_SPINLOCK_STATIC;
+        PRIVILEGED_DATA static portSPINLOCK_TYPE xISRSpinlock = portINIT_SPINLOCK_STATIC;
+    #endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
+


Suggested change

#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )

PRIVILEGED_DATA static portSPINLOCK_TYPE xTaskSpinlock = portINIT_SPINLOCK_STATIC;

PRIVILEGED_DATA static portSPINLOCK_TYPE xISRSpinlock = portINIT_SPINLOCK_STATIC;

#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */

#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )

PRIVILEGED_DATA static portSPINLOCK_TYPE xTimerTaskSpinlock = portINIT_SPINLOCK_STATIC;

PRIVILEGED_DATA static portSPINLOCK_TYPE xTimerISRSpinlock = portINIT_SPINLOCK_STATIC;

#endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */

Suggest to use different name so that the spinlock name is distinct in the kernel.

…herit() xTaskPriorityInherit() is called inside a critical section from queue.c. This commit moves the critical section into xTaskPriorityInherit(). Co-authored-by: Sudeep Mohanty <[email protected]>

Changed xPreemptionDisable to be a count rather than a pdTRUE/pdFALSE. This allows nested calls to vTaskPreemptionEnable(), where a yield only occurs when xPreemptionDisable is 0. Co-authored-by: Sudeep Mohanty <[email protected]>

Adds the required checks for granular locking port macros. Port Config: - portUSING_GRANULAR_LOCKS to enable granular locks - portCRITICAL_NESTING_IN_TCB should be disabled Granular Locking Port Macros: - Spinlocks - portSPINLOCK_TYPE - portINIT_SPINLOCK( pxSpinlock ) - portINIT_SPINLOCK_STATIC - Locking - portGET_SPINLOCK() - portRELEASE_SPINLOCK() Co-authored-by: Sudeep Mohanty <[email protected]>

- Updated prvCheckForRunStateChange() for granular locks - Updated vTaskSuspendAll() and xTaskResumeAll() - Now holds the xTaskSpinlock during kernel suspension - Increments/decrements xPreemptionDisable. Only yields when 0, thus allowing for nested suspensions across different data groups Co-authored-by: Sudeep Mohanty <[email protected]>

Updated critical section macros with granular locks. Some tasks.c API relied on their callers to enter critical sections. This assumption no longer works under granular locking. Critical sections added to the following functions: - `vTaskInternalSetTimeOutState()` - `xTaskIncrementTick()` - `vTaskSwitchContext()` - `xTaskRemoveFromEventList()` - `vTaskInternalSetTimeOutState()` - `eTaskConfirmSleepModeStatus()` - `xTaskPriorityDisinherit()` - `pvTaskIncrementMutexHeldCount()` Added missing suspensions to the following functions: - `vTaskPlaceOnEventList()` - `vTaskPlaceOnUnorderedEventList()` - `vTaskPlaceOnEventListRestricted()` Fixed the locking in vTaskSwitchContext() vTaskSwitchContext() must aquire both kernel locks, viz., task lock and ISR lock. This is because, vTaskSwitchContext() can be called from either task context or ISR context. Also, vTaskSwitchContext() must not alter the interrupt state prematurely. Co-authored-by: Sudeep Mohanty <[email protected]>

Updated queue.c to use granular locking - Added xTaskSpinlock and xISRSpinlock - Replaced critical section macros with data group critical section macros such as taskENTER/EXIT_CRITICAL/_FROM_ISR() with queueENTER/EXIT_CRITICAL_FROM_ISR(). - Added vQueueEnterCritical/FromISR() and vQueueExitCritical/FromISR() which map to the data group critical section macros. - Added prvLockQueueForTasks() and prvUnlockQueueForTasks() as the granular locking equivalents to prvLockQueue() and prvUnlockQueue() respectively Co-authored-by: Sudeep Mohanty <[email protected]>

Updated event_groups.c to use granular locking - Added xTaskSpinlock and xISRSpinlock - Replaced critical section macros with data group critical section macros such as taskENTER/EXIT_CRITICAL/_FROM_ISR() with event_groupsENTER/EXIT_CRITICAL/_FROM_ISR(). - Added vEventGroupsEnterCritical/FromISR() and vEventGroupsExitCriti/FromISR() functions that map to the data group critical section macros. - Added prvLockEventGroupForTasks() and prvUnlockEventGroupForTasks() to suspend the event group when executing non-deterministic code. - xEventGroupSetBits() and vEventGroupDelete() accesses the kernel data group directly. Thus, added vTaskSuspendAll()/xTaskResumeAll() to these fucntions. Co-authored-by: Sudeep Mohanty <[email protected]>

Updated stream_buffer.c to use granular locking - Added xTaskSpinlock and xISRSpinlock - Replaced critical section macros with data group critical section macros such as taskENTER/EXIT_CRITICAL/_FROM_ISR() with sbENTER/EXIT_CRITICAL_FROM_ISR(). - Added vStreambuffersEnterCritical/FromISR() and vStreambuffersExitCritical/FromISR() to map to the data group critical section macros. - Added prvLockStreamBufferForTasks() and prvUnlockStreamBufferForTasks() to suspend the stream buffer when executing non-deterministic code. Co-authored-by: Sudeep Mohanty <[email protected]>

Updated timers.c to use granular locking - Added xTaskSpinlock and xISRSpinlock - Replaced critical section macros with data group critical section macros such as taskENTER/EXIT_CRITICAL() with tmrENTER/EXIT_CRITICAL(). - Added vTimerEnterCritical() and vTimerExitCritical() to map to the data group critical section macros. Co-authored-by: Sudeep Mohanty <[email protected]>

Co-authored-by: Sudeep Mohanty <[email protected]>

sonarqubecloud · 2025-08-05T09:18:40Z

Quality Gate passed

Issues
22 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.1% Duplication on New Code

See analysis details on SonarQube Cloud

chinglee-iot · 2025-08-07T06:26:50Z

queue.c

+    #define queueLOCK( pxQueue )                                            \
+    do {                                                                    \
+        vTaskPreemptionDisable( NULL );                                     \
+        prvLockQueue( ( pxQueue ) );                                        \
+        portGET_SPINLOCK( portGET_CORE_ID(), &( pxQueue->xTaskSpinlock ) ); \
+    } while( 0 )


#define queueLOCK( pxQueue ) \ do { \ vTaskPreemptionDisable( NULL ); \ prvLockQueue( ( pxQueue ) ); \ // Potential gap here since no lock is acquired \ portGET_SPINLOCK( portGET_CORE_ID(), &( pxQueue->xTaskSpinlock ) ); \ } while( 0 )

There is a potential gap between prvLockQueue and portGET_SPINLOCK. Suggest we swap the order.

Suggest we consider to add the following macros in task.h and reuse them in the implementation.

#define taskDATA_GROUP_LOCK( pxTaskSpinlock ) \ do{ \ vTaskPreemptionDisable( NULL ); \ portGET_SPINLOCK( portGET_CORE_ID(), (portSPINLOCK_TYPE *)(pxTaskSpinlock) ); \ } while( 0 ) #define taskDATA_GROUP_UNLOCK( pxTaskSpinlock ) \ do{ \ portRELEASE_SPINLOCK( portGET_CORE_ID(), (portSPINLOCK_TYPE *)(pxTaskSpinlock) ); \ vTaskPreemptionEnable( NULL ); \ } while( 0 )

Suggested change

#define queueLOCK( pxQueue ) \

do { \

vTaskPreemptionDisable( NULL ); \

prvLockQueue( ( pxQueue ) ); \

portGET_SPINLOCK( portGET_CORE_ID(), &( pxQueue->xTaskSpinlock ) ); \

} while( 0 )

#define queueLOCK( pxQueue ) \

do { \

taskDATA_GROUP_LOCK( &( pxQueue->xTaskSpinlock ) ); \

prvLockQueue( ( pxQueue ) ); \

} while( 0 )

chinglee-iot · 2025-08-07T06:44:21Z

queue.c

-                        taskENTER_CRITICAL();
-                        {
-                            xInheritanceOccurred = xTaskPriorityInherit( pxQueue->u.xSemaphore.xMutexHolder );
-                        }
-                        taskEXIT_CRITICAL();


We try to enter the kernel data group critical section only in tasks.c. So we move the taskENTER/EXIT_CRITICAL() here to the tasks.c.

We should remove the "#if ( portUSING_GRANULAR_LOCKS == 1 )" check in xTaskPriorityInherit implementation.

BaseType_t xTaskPriorityInherit( TaskHandle_t const pxMutexHolder ) { TCB_t * const pxMutexHolderTCB = pxMutexHolder; BaseType_t xReturn = pdFALSE; traceENTER_xTaskPriorityInherit( pxMutexHolder ); #if ( portUSING_GRANULAR_LOCKS == 1 ) kernelENTER_CRITICAL(); #endif ... #if ( portUSING_GRANULAR_LOCKS == 1 ) kernelEXIT_CRITICAL(); #endif traceRETURN_xTaskPriorityInherit( xReturn ); return xReturn; }

chinglee-iot · 2025-08-08T02:42:36Z

stream_buffer.c

+    #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
+    static void prvLockStreamBufferForTasks( StreamBuffer_t * const pxStreamBuffer )
+    {
+        /* Disable preemption so that the current task cannot be preempted by another task */
+        vTaskPreemptionDisable( NULL );
+
+        /* Keep holding xTaskSpinlock after unlocking the data group to prevent tasks
+         * on other cores from accessing the stream buffer while it is suspended. */
+        portGET_SPINLOCK( portGET_CORE_ID(), &( pxStreamBuffer->xTaskSpinlock ) );
+    }
+    #endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
+/*-----------------------------------------------------------*/
+
+    #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
+    static void prvUnlockStreamBufferForTasks( StreamBuffer_t * const pxStreamBuffer )
+    {
+        /* Release the previously held task spinlock */
+        portRELEASE_SPINLOCK( portGET_CORE_ID(), &( pxStreamBuffer->xTaskSpinlock ) );
+
+        /* Re-enable preemption */
+        vTaskPreemptionEnable( NULL );
+    }
+    #endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */
+/*-----------------------------------------------------------*/


Similar suggestion to use taskDATA_GROUP_LOCK/taskDATA_GROUP_UNLOCK for these functions.

chinglee-iot · 2025-08-11T07:58:04Z

event_groups.c

+            vTaskPreemptionEnable( NULL );
+
+            /* Yield if preemption was re-enabled*/
+            if( xTaskUnlockCanYield() == pdTRUE )


xTaskUnlockCanYield() should be called in critical section. We may create another internal API to return the already yielded information when preemption enabled.
For example,

Basetype_t xCurrentTaskPreemptionEnable( void );

chinglee-iot · 2025-08-12T05:51:36Z

include/FreeRTOS.h

+    #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
+        #define portTICK_TYPE_ENTER_CRITICAL()                  portENTER_CRITICAL_DATA_GROUP( &xTaskSpinlock, &xISRSpinlock )
+        #define portTICK_TYPE_EXIT_CRITICAL()                   portEXIT_CRITICAL_DATA_GROUP( &xTaskSpinlock, &xISRSpinlock )
+    #else /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */


Suggested change

#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )

#define portTICK_TYPE_ENTER_CRITICAL() portENTER_CRITICAL_DATA_GROUP( &xTaskSpinlock, &xISRSpinlock )

#define portTICK_TYPE_EXIT_CRITICAL() portEXIT_CRITICAL_DATA_GROUP( &xTaskSpinlock, &xISRSpinlock )

#else /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */

#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )

#define portTICK_TYPE_ENTER_CRITICAL() vTaskEnterCritical()

#define portTICK_TYPE_EXIT_CRITICAL() VtaskExitCritical()

#else /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */

We should still use kernel data group critical section here.

chinglee-iot · 2025-08-12T05:56:12Z

include/task.h

+#if ( portUSING_GRANULAR_LOCKS == 1 )
+    #define taskDATA_GROUP_ENTER_CRITICAL( pxTaskSpinlock, pxISRSpinlock )                \
+    do {                                                                                  \
+        /* Disable preemption to avoid task state changes during the critical section. */ \
+        vTaskPreemptionDisable( NULL );                                                   \
+        {                                                                                 \
+            const BaseType_t xCoreID = ( BaseType_t ) portGET_CORE_ID();                  \
+            /* Task spinlock is always taken first */                                     \
+            portGET_SPINLOCK( xCoreID, ( portSPINLOCK_TYPE * ) pxTaskSpinlock );          \
+            /* Disable interrupts */                                                      \
+            portDISABLE_INTERRUPTS();                                                     \
+            /* Take the ISR spinlock next */                                              \
+            portGET_SPINLOCK( xCoreID, ( portSPINLOCK_TYPE * ) pxISRSpinlock );           \
+            /* Increment the critical nesting count */                                    \
+            portINCREMENT_CRITICAL_NESTING_COUNT( xCoreID );                              \
+        }                                                                                 \
+    } while( 0 )
+#endif /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */


Suggested change

#if ( portUSING_GRANULAR_LOCKS == 1 )

#define taskDATA_GROUP_ENTER_CRITICAL( pxTaskSpinlock, pxISRSpinlock ) \

do { \

/* Disable preemption to avoid task state changes during the critical section. */ \

vTaskPreemptionDisable( NULL ); \

{ \

const BaseType_t xCoreID = ( BaseType_t ) portGET_CORE_ID(); \

/* Task spinlock is always taken first */ \

portGET_SPINLOCK( xCoreID, ( portSPINLOCK_TYPE * ) pxTaskSpinlock ); \

/* Disable interrupts */ \

portDISABLE_INTERRUPTS(); \

/* Take the ISR spinlock next */ \

portGET_SPINLOCK( xCoreID, ( portSPINLOCK_TYPE * ) pxISRSpinlock ); \

/* Increment the critical nesting count */ \

portINCREMENT_CRITICAL_NESTING_COUNT( xCoreID ); \

} \

} while( 0 )

#endif /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */

#if ( portUSING_GRANULAR_LOCKS == 1 )

#define taskDATA_GROUP_ENTER_CRITICAL( pxTaskSpinlock, pxISRSpinlock ) \

do { \

/* Disable preemption to avoid task state changes during the critical section. */ \

vTaskPreemptionDisable( NULL ); \

{ \

const BaseType_t xCoreID = ( BaseType_t ) portGET_CORE_ID(); \

/* Task spinlock is always taken first */ \

portGET_SPINLOCK( xCoreID, ( portSPINLOCK_TYPE * ) ( pxTaskSpinlock ) ); \

/* Disable interrupts */ \

portDISABLE_INTERRUPTS(); \

/* Take the ISR spinlock next */ \

portGET_SPINLOCK( xCoreID, ( portSPINLOCK_TYPE * ) ( pxISRSpinlock ) ); \

/* Increment the critical nesting count */ \

portINCREMENT_CRITICAL_NESTING_COUNT( xCoreID ); \

} \

} while( 0 )

#endif /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */

chinglee-iot · 2025-08-12T05:57:38Z

include/task.h

+#if ( portUSING_GRANULAR_LOCKS == 1 )
+    #define taskDATA_GROUP_EXIT_CRITICAL( pxTaskSpinlock, pxISRSpinlock )        \
+    do {                                                                         \
+        const BaseType_t xCoreID = ( BaseType_t ) portGET_CORE_ID();             \
+        configASSERT( portGET_CRITICAL_NESTING_COUNT( xCoreID ) > 0U );          \
+        /* Release the ISR spinlock */                                           \
+        portRELEASE_SPINLOCK( xCoreID, ( portSPINLOCK_TYPE * ) pxISRSpinlock );  \
+        /* Release the task spinlock */                                          \
+        portRELEASE_SPINLOCK( xCoreID, ( portSPINLOCK_TYPE * ) pxTaskSpinlock ); \
+        /* Decrement the critical nesting count */                               \
+        portDECREMENT_CRITICAL_NESTING_COUNT( xCoreID );                         \
+        /* Enable interrupts only if the critical nesting count is 0 */          \
+        if( portGET_CRITICAL_NESTING_COUNT( xCoreID ) == 0 )                     \
+        {                                                                        \
+            portENABLE_INTERRUPTS();                                             \
+        }                                                                        \
+        else                                                                     \
+        {                                                                        \
+            mtCOVERAGE_TEST_MARKER();                                            \
+        }                                                                        \
+        /* Re-enable preemption */                                               \
+        vTaskPreemptionEnable( NULL );                                           \
+    } while( 0 )
+#endif /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */


Suggested change

#if ( portUSING_GRANULAR_LOCKS == 1 )

#define taskDATA_GROUP_EXIT_CRITICAL( pxTaskSpinlock, pxISRSpinlock ) \

do { \

const BaseType_t xCoreID = ( BaseType_t ) portGET_CORE_ID(); \

configASSERT( portGET_CRITICAL_NESTING_COUNT( xCoreID ) > 0U ); \

/* Release the ISR spinlock */ \

portRELEASE_SPINLOCK( xCoreID, ( portSPINLOCK_TYPE * ) pxISRSpinlock ); \

/* Release the task spinlock */ \

portRELEASE_SPINLOCK( xCoreID, ( portSPINLOCK_TYPE * ) pxTaskSpinlock ); \

/* Decrement the critical nesting count */ \

portDECREMENT_CRITICAL_NESTING_COUNT( xCoreID ); \

/* Enable interrupts only if the critical nesting count is 0 */ \

if( portGET_CRITICAL_NESTING_COUNT( xCoreID ) == 0 ) \

{ \

portENABLE_INTERRUPTS(); \

} \

else \

{ \

mtCOVERAGE_TEST_MARKER(); \

} \

/* Re-enable preemption */ \

vTaskPreemptionEnable( NULL ); \

} while( 0 )

#endif /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */

#if ( portUSING_GRANULAR_LOCKS == 1 )

#define taskDATA_GROUP_EXIT_CRITICAL( pxTaskSpinlock, pxISRSpinlock ) \

do { \

const BaseType_t xCoreID = ( BaseType_t ) portGET_CORE_ID(); \

configASSERT( portGET_CRITICAL_NESTING_COUNT( xCoreID ) > 0U ); \

/* Release the ISR spinlock */ \

portRELEASE_SPINLOCK( xCoreID, ( portSPINLOCK_TYPE * ) ( pxISRSpinlock ) ); \

/* Release the task spinlock */ \

portRELEASE_SPINLOCK( xCoreID, ( portSPINLOCK_TYPE * ) ( pxTaskSpinlock ) ); \

/* Decrement the critical nesting count */ \

portDECREMENT_CRITICAL_NESTING_COUNT( xCoreID ); \

/* Enable interrupts only if the critical nesting count is 0 */ \

if( portGET_CRITICAL_NESTING_COUNT( xCoreID ) == 0 ) \

{ \

portENABLE_INTERRUPTS(); \

} \

else \

{ \

mtCOVERAGE_TEST_MARKER(); \

} \

/* Re-enable preemption */ \

vTaskPreemptionEnable( NULL ); \

} while( 0 )

#endif /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */

chinglee-iot · 2025-08-12T05:58:23Z

include/task.h

+#if ( portUSING_GRANULAR_LOCKS == 1 )
+    #define taskDATA_GROUP_EXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, pxISRSpinlock ) \
+    do {                                                                                   \
+        const BaseType_t xCoreID = ( BaseType_t ) portGET_CORE_ID();                       \
+        configASSERT( portGET_CRITICAL_NESTING_COUNT( xCoreID ) > 0U );                    \
+        /* Decrement the critical nesting count */                                         \
+        portDECREMENT_CRITICAL_NESTING_COUNT( xCoreID );                                   \
+        /* Release the ISR spinlock */                                                     \
+        portRELEASE_SPINLOCK( xCoreID, ( portSPINLOCK_TYPE * ) pxISRSpinlock );            \
+        if( portGET_CRITICAL_NESTING_COUNT( xCoreID ) == 0 )                               \
+        {                                                                                  \
+            portCLEAR_INTERRUPT_MASK_FROM_ISR( uxSavedInterruptStatus );                   \
+        }                                                                                  \
+    } while( 0 )
+#endif /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */


Suggested change

#if ( portUSING_GRANULAR_LOCKS == 1 )

#define taskDATA_GROUP_EXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, pxISRSpinlock ) \

do { \

const BaseType_t xCoreID = ( BaseType_t ) portGET_CORE_ID(); \

configASSERT( portGET_CRITICAL_NESTING_COUNT( xCoreID ) > 0U ); \

/* Decrement the critical nesting count */ \

portDECREMENT_CRITICAL_NESTING_COUNT( xCoreID ); \

/* Release the ISR spinlock */ \

portRELEASE_SPINLOCK( xCoreID, ( portSPINLOCK_TYPE * ) pxISRSpinlock ); \

if( portGET_CRITICAL_NESTING_COUNT( xCoreID ) == 0 ) \

{ \

portCLEAR_INTERRUPT_MASK_FROM_ISR( uxSavedInterruptStatus ); \

} \

} while( 0 )

#endif /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */

#if ( portUSING_GRANULAR_LOCKS == 1 )

#define taskDATA_GROUP_EXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, pxISRSpinlock ) \

do { \

const BaseType_t xCoreID = ( BaseType_t ) portGET_CORE_ID(); \

configASSERT( portGET_CRITICAL_NESTING_COUNT( xCoreID ) > 0U ); \

/* Decrement the critical nesting count */ \

portDECREMENT_CRITICAL_NESTING_COUNT( xCoreID ); \

/* Release the ISR spinlock */ \

portRELEASE_SPINLOCK( xCoreID, ( portSPINLOCK_TYPE * ) ( pxISRSpinlock ) ); \

if( portGET_CRITICAL_NESTING_COUNT( xCoreID ) == 0 ) \

{ \

portCLEAR_INTERRUPT_MASK_FROM_ISR( uxSavedInterruptStatus ); \

} \

} while( 0 )

#endif /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */

chinglee-iot · 2025-08-12T06:02:05Z

stream_buffer.c

+    #if ( portUSING_GRANULAR_LOCKS == 1 )
+        #define sbENTER_CRITICAL( pxStreamBuffer )                                      taskDATA_GROUP_ENTER_CRITICAL( &pxStreamBuffer->xTaskSpinlock, &pxStreamBuffer->xISRSpinlock )
+        #define sbENTER_CRITICAL_FROM_ISR( pxStreamBuffer, puxSavedInterruptStatus )    taskDATA_GROUP_ENTER_CRITICAL_FROM_ISR( &pxStreamBuffer->xISRSpinlock, puxSavedInterruptStatus )
+        #define sbEXIT_CRITICAL( pxStreamBuffer )                                       taskDATA_GROUP_EXIT_CRITICAL( &pxStreamBuffer->xTaskSpinlock, &pxStreamBuffer->xISRSpinlock )
+        #define sbEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, pxStreamBuffer )      taskDATA_GROUP_EXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, &pxStreamBuffer->xISRSpinlock )
+    #else /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */
+        #define sbENTER_CRITICAL( pxStreamBuffer )                                      taskENTER_CRITICAL();
+        #define sbENTER_CRITICAL_FROM_ISR( pxStreamBuffer, puxSavedInterruptStatus )    do { *( puxSavedInterruptStatus ) = taskENTER_CRITICAL_FROM_ISR(); } while( 0 )
+        #define sbEXIT_CRITICAL( pxStreamBuffer )                                       taskEXIT_CRITICAL();
+        #define sbEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, pxStreamBuffer )      taskEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus );
+    #endif /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */


Suggested change

#if ( portUSING_GRANULAR_LOCKS == 1 )

#define sbENTER_CRITICAL( pxStreamBuffer ) taskDATA_GROUP_ENTER_CRITICAL( &pxStreamBuffer->xTaskSpinlock, &pxStreamBuffer->xISRSpinlock )

#define sbENTER_CRITICAL_FROM_ISR( pxStreamBuffer, puxSavedInterruptStatus ) taskDATA_GROUP_ENTER_CRITICAL_FROM_ISR( &pxStreamBuffer->xISRSpinlock, puxSavedInterruptStatus )

#define sbEXIT_CRITICAL( pxStreamBuffer ) taskDATA_GROUP_EXIT_CRITICAL( &pxStreamBuffer->xTaskSpinlock, &pxStreamBuffer->xISRSpinlock )

#define sbEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, pxStreamBuffer ) taskDATA_GROUP_EXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, &pxStreamBuffer->xISRSpinlock )

#else /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */

#define sbENTER_CRITICAL( pxStreamBuffer ) taskENTER_CRITICAL();

#define sbENTER_CRITICAL_FROM_ISR( pxStreamBuffer, puxSavedInterruptStatus ) do { *( puxSavedInterruptStatus ) = taskENTER_CRITICAL_FROM_ISR(); } while( 0 )

#define sbEXIT_CRITICAL( pxStreamBuffer ) taskEXIT_CRITICAL();

#define sbEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, pxStreamBuffer ) taskEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus );

#endif /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */

#if ( portUSING_GRANULAR_LOCKS == 1 )

#define sbENTER_CRITICAL( pxStreamBuffer ) taskDATA_GROUP_ENTER_CRITICAL( &( ( pxStreamBuffer )->xTaskSpinlock ), &( ( pxStreamBuffer )->xISRSpinlock ) )

#define sbENTER_CRITICAL_FROM_ISR( pxStreamBuffer, puxSavedInterruptStatus ) taskDATA_GROUP_ENTER_CRITICAL_FROM_ISR( &( ( pxStreamBuffer )->xISRSpinlock ), puxSavedInterruptStatus )

#define sbEXIT_CRITICAL( pxStreamBuffer ) taskDATA_GROUP_EXIT_CRITICAL( &( ( pxStreamBuffer )->xTaskSpinlock ), &( ( pxStreamBuffer )->xISRSpinlock ) )

#define sbEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, pxStreamBuffer ) taskDATA_GROUP_EXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, &( ( pxStreamBuffer )->xISRSpinlock ) )

#else /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */

#define sbENTER_CRITICAL( pxStreamBuffer ) taskENTER_CRITICAL();

#define sbENTER_CRITICAL_FROM_ISR( pxStreamBuffer, puxSavedInterruptStatus ) do { *( puxSavedInterruptStatus ) = taskENTER_CRITICAL_FROM_ISR(); } while( 0 )

#define sbEXIT_CRITICAL( pxStreamBuffer ) taskEXIT_CRITICAL();

#define sbEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus, pxStreamBuffer ) taskEXIT_CRITICAL_FROM_ISR( uxSavedInterruptStatus );

#endif /* #if ( portUSING_GRANULAR_LOCKS == 1 ) */

chinglee-iot · 2025-08-12T06:09:04Z

tasks.c

@@ -2644,7 +2818,7 @@ static void prvInitialiseNewTask( TaskFunction_t pxTaskCode,

        traceENTER_uxTaskPriorityGet( xTask );

-        portBASE_TYPE_ENTER_CRITICAL();
+        kernelENTER_CRITICAL();


portBASE_TYPE_ENTER_CRITICAL() and portBASE_TYPE_EXIT_CRITICAL() is still required for single core port.

chinglee-iot · 2025-08-12T06:18:46Z

tasks.c

+                        #if ( configUSE_TASK_PREEMPTION_DISABLE == 1 )
+                            && ( pxCurrentTCBs[ xCoreID ]->uxPreemptionDisable == 0U )
+                        #endif


Probably we can consider to remove the preemption disable check here.

If xTaskResumeAll() is called with preemption disabled, but the xYieldPendings is set. The caller function can not yield itself as well since preemption is disabled. When preemption is enabled, xYieldPendings will be handled automatically.

Return value of xTaskResumeAll() should not be used when preemption is disabled in the caller function. This will cause scheduler fail to scheduler correct tasks. Similar situation is like calling FreeRTOS API in critical section should be handled carefully.

chinglee-iot · 2025-08-12T06:20:49Z

tasks.c

-                {
-                    mtCOVERAGE_TEST_MARKER();
-                }
+                #if ( configUSE_TASK_PREEMPTION_DISABLE == 1 )


configUSE_TASK_PREEMPTION_DISABLE should not be used with single core. We probably don't need the following logic.

chinglee-iot · 2025-08-12T06:31:01Z

tasks.c

+                kernelGET_TASK_LOCK( xCoreID );
+                kernelGET_ISR_LOCK( xCoreID );


The only difference between granular lock and non-granular lock version is to check critical nesting count before acquiring the spinlocks. Can we consider to use compile macro and add comment for explanation instead of clone a new function here?

chinglee-iot · 2025-08-12T07:08:29Z

tasks.c

+                    BaseType_t xYieldCurrentTask;
+
+                    /* Get the xYieldPending stats inside the critical section. */
+                    xYieldCurrentTask = xTaskUnlockCanYield();


Acquiring the yield pending should be performed in critical section. We should move this before release spinlocks.

And also thank you for the logic inside xTaskUnlockCanYield(). I use the following test case to identify that there will be a redundant context switch without checking scheduler suspended.

void test_task_scheduler_suspend_critical_section( void ) { TaskHandle_t xMainTaskHandle; /* Create high priority main task. */ xTaskCreate( vSmpTestTask, "SMP Task", configMINIMAL_STACK_SIZE, NULL, 3, &xMainTaskHandle ); /* Start the scheduler. */ vTaskStartScheduler(); vTaskSuspendAll(); vTaskEnterCritical(); /* Request the task to yield. */ vTaskYieldWithinAPI(); /* The scheduler is still suspended. Leaving the critical section should not * request the core for context switch. */ vTaskExitCritical(); /* Context switch should happens when scheduler is resumed. */ ( void ) xTaskResumeAll(); }

chinglee-iot · 2025-08-12T07:39:21Z

tasks.c

+
+#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )
+
+    BaseType_t xTaskUnlockCanYield( void )


Currently, this function is used three times in the implementation

event_groups.c - We can consider to add a internal function 'xCurrentTaskEnablePreemption()' to return the already yielding information.

tasks.c exit critical section - We can use move the implementation here to exit critical section function.

This function should be called in a consecutive critical section. Suggest we consider to remove this function.

sudeep-mohanty marked this pull request as draft October 15, 2024 08:25

sudeep-mohanty force-pushed the feature/smp_granular_locks_v4 branch 9 times, most recently from 0159a4a to 5bf8c33 Compare October 30, 2024 10:26

sudeep-mohanty marked this pull request as ready for review October 30, 2024 10:34

sudeep-mohanty requested a review from a team as a code owner October 30, 2024 10:34

sudeep-mohanty force-pushed the feature/smp_granular_locks_v4 branch from 5bf8c33 to 9f8acc7 Compare October 31, 2024 09:25

sudeep-mohanty force-pushed the feature/smp_granular_locks_v4 branch 2 times, most recently from c79962d to f50d476 Compare October 31, 2024 12:43

sudeep-mohanty mentioned this pull request Nov 1, 2024

change(freertos/smp/granular_locks): Updated prvYieldCore() coverage tests FreeRTOS/FreeRTOS#1292

Closed

1 task

sudeep-mohanty force-pushed the feature/smp_granular_locks_v4 branch 4 times, most recently from 70eb466 to edc1c98 Compare November 4, 2024 15:38

sudeep-mohanty marked this pull request as draft November 15, 2024 14:25

chinglee-iot reviewed Dec 9, 2024

View reviewed changes

timers.c Outdated Show resolved Hide resolved

chinglee-iot reviewed Jul 21, 2025

View reviewed changes

Dazza0 and others added 11 commits August 3, 2025 11:14

refactor(freertos/smp): Move critical sections inside xTaskPriorityIn…

11bc74c

…herit() xTaskPriorityInherit() is called inside a critical section from queue.c. This commit moves the critical section into xTaskPriorityInherit(). Co-authored-by: Sudeep Mohanty <[email protected]>

feat(freertos/smp): Add Granular Locking V4 proposal documents

ec3c41e

Co-authored-by: Sudeep Mohanty <[email protected]>

feat(freertos-smp): Light Weight Preemption Disable Locks

40a991d

sudeep-mohanty force-pushed the feature/smp_granular_locks_v4 branch 2 times, most recently from 190fc03 to 40a991d Compare August 5, 2025 09:16

fix(freertos-smp): Stop unconditional yielding in vTaskPreemptionEnable

7df6202

chinglee-iot reviewed Aug 7, 2025

View reviewed changes

chinglee-iot reviewed Aug 8, 2025

View reviewed changes

chinglee-iot reviewed Aug 11, 2025

View reviewed changes

chinglee-iot reviewed Aug 12, 2025

View reviewed changes

		kernelGET_TASK_LOCK( xCoreID );
		kernelGET_ISR_LOCK( xCoreID );


		#if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) )

		BaseType_t xTaskUnlockCanYield( void )

Feature/smp granular locks v4 #1154

Are you sure you want to change the base?

Feature/smp granular locks v4 #1154

Conversation

sudeep-mohanty commented Oct 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Test Steps

TODO

Checklist:

Related Issue

Uh oh!

sudeep-mohanty commented Oct 9, 2024

Uh oh!

rawalexe commented Oct 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rawalexe commented Oct 14, 2024

Uh oh!

sudeep-mohanty commented Oct 15, 2024

Uh oh!

ActoryOu commented Oct 31, 2024

Uh oh!

sudeep-mohanty commented Oct 31, 2024

Uh oh!

sudeep-mohanty commented Nov 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonarqubecloud bot commented Nov 4, 2024

Quality Gate passed

Uh oh!

Uh oh!

Uh oh!

chinglee-iot Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Aug 5, 2025

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chinglee-iot Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chinglee-iot Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chinglee-iot Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chinglee-iot Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

sudeep-mohanty commented Oct 9, 2024 •

edited

Loading

rawalexe commented Oct 9, 2024 •

edited

Loading

sudeep-mohanty commented Nov 1, 2024 •

edited

Loading

chinglee-iot Jul 21, 2025 •

edited

Loading

chinglee-iot Aug 11, 2025 •

edited

Loading

chinglee-iot Aug 12, 2025 •

edited

Loading

chinglee-iot Aug 12, 2025 •

edited

Loading

chinglee-iot Aug 12, 2025 •

edited

Loading

chinglee-iot Aug 12, 2025 •

edited

Loading